fix(ssr): hoist export to handle cyclic import better #18983
Conversation
hi-ogawa
force-pushed
the
fix-ssr-transform-hoist-export
branch
from
ff8cf4b to
3cc86da
Compare
|
/ecosystem-ci run |
commit: |
|
📝 Ran ecosystem CI on
✅ analogjs, astro, ladle, previewjs, quasar, qwik, rakkas, storybook, sveltekit, unocss, vite-environment-examples, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-swc, vite-plugin-vue, vite-setup-catalogue, vitepress, vuepress |
vitest.config.ts
Outdated
| // TODO: | ||
| // importing non entry file can be broken due to cyclic import e.g. | ||
| // pnpm exec tsx packages/vite/src/node/server/index.ts | ||
| setupFiles: ['./packages/vite/src/node/index.ts'], |
There was a problem hiding this comment.
Interestingly, export hoisting can make cyclic import handling stricter as seen by:
$ pnpm test-unit packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts
FAIL packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts [ packages/vite/src/node/ssr/runtime/__tests__/server-runtime.spec.ts ]
ReferenceError: Cannot access 'serverConfigDefaults' before initialization
❯ Module.get [as serverConfigDefaults] packages/vite/src/node/server/index.ts:4:116but this case might be legitimate since this is the same error as tsx:
$ pnpm exec tsx packages/vite/src/node/server/index.ts
/home/hiroshi/code/others/vite/packages/vite/src/node/config.ts:648
server: serverConfigDefaults,
^
ReferenceError: Cannot access 'serverConfigDefaults' before initializationI'd imagine previously these were undefined (still wrong but no hard error) since __vite_ssr_import_x_.serverConfigDefaults getter was not defined. But now this hits uninitialized const serverConfigDefaults access due to getter.
Just temporarily, I added this silly setupFiles, so that modules will be initialized in a known safe order.
hi-ogawa
force-pushed
the
fix-ssr-transform-hoist-export
branch
from
4e35256 to
2245aaa
Compare
| // it throws a same error as browser case, | ||
| // but it doesn't auto reload and it calls `hot.accept` called with `undefined` | ||
| await untilUpdated(() => el(), '') |
There was a problem hiding this comment.
It turns out the error now became same as playground/hmr but module runner doesn't auto-reload, so the existing test failed.
ReferenceError: Cannot access 'c' before initialization
I adjust this test case for now to make it pass.
Description
I was checking webpack's output (see my notes in https://github.com/hi-ogawa/reproductions/tree/main/cyclic-import) and the one idea for handling cyclic import better might be to hoist export definition before import. (Also linking potentially related conversation on where to emit export on rolldown rolldown/rolldown#2974).
todo
vitest: broken source map?(Vitest issue test: skip broken source map test vitest-dev/vitest#7096)Object.definePropertyearly